Maybe you can take a look at #515 and cherry-pick the instrumentation and sentry-tracing layer.

I'd rather add instrumentation and structure in a second PR tbh. As for the sentry-tracing layer, is there anything more that needs to be done other than add a SentryLayer to the subscriber?

So before the most recent commit, some python tests were failing with a message about how they couldn't find a port or some such. Turns out, these tests start a symbolicator process and then wait for a specific log line from actix_web from which they parse the port that symbolicator is listening on. And since I hadn't set up log integration, the actix_web logs were silently discarded and the tests never found out the port 😅

Other than that, please reach out to ops about whether we actually use these logs (json formatted?) somewhere. If we might change the format or if we need to keep backwards compatibility.

Yes we use JSON formatted logs in production, you can get to them from the symbolicator metrics dashboard by following the "stackdriver logs" link.

The most recent commit (5142030) solves the problem of keeping log output exactly the same in the simplest way: instead of using a tracing::Subscriber, we keep the logger and convert all tracing events to log records. This has the advantage of being very easy (really just add the tracing dependency with the log feature enabled and replace all the log invocations), but the disadvantage that it doesn't get rid of log entirely. Moreover, once we want to use tracing in more sophisticated ways, we will have to use a subscriber anyway. Thoughts?

I preferred the version where you removed log entirely. We should be able to make sure the tracing subscriber json output includes at least the same fields as we had before and then we're good.

I think in the future it would be nice to also start using the tracing macros with extra fields which would show up in the json log and make our log search much more useful.

@flub Is it clear that there's no harm in adding extra fields to the json logs?

@flub Is it clear that there's no harm in adding extra fields to the json logs?

I'm like 95% sure... 😄 @oioki might be able to give a better answer?

Codecov Report

Merging #534 (3775d1c) into master (0e99a5e) will increase coverage by 0.18%.
The diff coverage is 27.77%.

@@            Coverage Diff             @@
##           master     #534      +/-   ##
==========================================
+ Coverage   74.09%   74.27%   +0.18%     
==========================================
  Files          47       47              
  Lines       10434    10419      -15     
==========================================
+ Hits         7731     7739       +8     
+ Misses       2703     2680      -23     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e99a5e...3775d1c. Read the comment docs.

Alright, I think this is finally ready for serious consideration. JSON log output doesn't line up exactly, but after talking to @oioki we think it shouldn't be a problem.
Old log line:

{"timestamp":"2022-01-25T12:44:44.202043Z","level":"INFO","logger":"symbolicator::server","message":"Starting server on 127.0.0.1:3021","module_path":"symbolicator::server","filename":"crates/symbolicator/src/server.rs","lineno":43}

New log line:

{"timestamp":"2022-01-25T12:52:19.015333Z","level":"INFO","message":"Starting server on 127.0.0.1:3021","target":"symbolicator::server","filename":"crates/symbolicator/src/server.rs","line_number":43}

agree the json differences are fine. let's merge this 🎉